fix(api): resolve infinite loading on trace/service fetch errors #3079 by Champbreed · Pull Request #3590 · jaegertracing/jaeger-ui

Champbreed · 2026-03-11T07:14:18Z

This PR resolves the infinite loading state observed when the Jaeger v3 API returns an error (both 4xx/5xx and 200 OK with an error body).

Changes:

Modified fetchWithTimeout to explicitly throw an Error when !response.ok, ensuring React Query transitions to an error state.
Implemented internal error checking in fetchServices and fetchSpanNames to catch "hidden" errors in the JSON body (closing the blind spot identified in fix(bug): Handle errors in response body with status code is 200 #3161).
Verified: Confirmed with a 404 response for non-existent Trace IDs (now displays error UI instead of spinning).

Fixes #3079.

Copilot

Pull request overview

This PR addresses an infinite loading state in the Jaeger UI when the v3 API returns failures by ensuring fetch errors propagate correctly into React Query’s error state.

Changes:

Updated the v3 client to detect “hidden” API errors in a 200 OK JSON body (errors array) for services and operations endpoints.
Changed fetchWithTimeout() to throw on non-2xx responses instead of returning a non-OK Response.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Implement detailed error extraction in fetchWithTimeout - Standardize error aggregation in fetchServices and fetchSpanNames - Clean up comments and improve contextual error messages - Fixes jaegertracing#3079 Signed-off-by: Simon Essien <champbreed1@gmail.com>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-11T08:18:04Z

+      if (!response.ok) {
+        let errorDetail = response.statusText;
+        try {
+          // Attempt to extract the specific backend error (e.g., "trace not found")
+          const errorBody = await response.json();
+          if (errorBody?.errors?.length > 0) {
+            errorDetail = errorBody.errors.map((e: any) => e.msg).join(', ');
+          }
+        } catch {
+          // If the body isn't JSON, we just use the default statusText
+        }
+
+        throw new Error(`HTTP ${response.status}: ${errorDetail}`);
+      }


This change alters the error handling contract (non-OK responses now throw from fetchWithTimeout, and 200 OK responses with errors now throw). The existing client.test.ts suite has cases asserting the previous error messages/paths and should be updated, and new tests should be added for the "200 with errors array" case for both fetchServices and fetchSpanNames to prevent regressions.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-11T09:16:06Z

  async fetchServices(): Promise<string[]> {
    const response = await this.fetchWithTimeout(`${this.apiRoot}/services`);
-    if (!response.ok) {
-      throw new Error(`Failed to fetch services: ${response.status} ${response.statusText}`);
-    }
+
+    // Parse once. Jaeger v3 may return 200 OK with an errors array in the body.
    const data = await response.json();



fetchWithTimeout() now throws on non-2xx responses, but fetchServices() no longer wraps those errors. As a result, callers/UI will likely show a generic message like "404 Not Found" instead of a contextual "Failed to fetch services: ...". Consider catching errors from fetchWithTimeout() here and rethrowing with endpoint context (preserving the original error message as the cause).

Copilot · 2026-03-11T09:16:07Z

  async fetchSpanNames(service: string): Promise<{ name: string; spanKind: string }[]> {
    const response = await this.fetchWithTimeout(
      `${this.apiRoot}/operations?service=${encodeURIComponent(service)}`
    );
-    if (!response.ok) {
-      throw new Error(
-        `Failed to fetch span names for service "${service}": ${response.status} ${response.statusText}`
-      );
-    }
+
+    // Jaeger v3 may return HTTP 200 with an 'errors' array in the response body; treat that as a failure.
    const data = await response.json();



Same as fetchServices(): since fetchWithTimeout() throws for non-OK responses, fetchSpanNames() will now surface non-contextual errors (e.g. "500 Internal Server Error") without indicating which service/endpoint failed. Consider wrapping/rethrowing with service context (and optionally keeping the original error as cause) so the UI error text remains actionable.

Copilot · 2026-03-11T09:16:07Z

+      if (!response.ok) {
+        let errorDetail = response.statusText;
+        try {
+          const errorBody = await response.json();
+          // Safely check if errors is an array before mapping
+          if (errorBody?.errors && Array.isArray(errorBody.errors) && errorBody.errors.length > 0) {
+            errorDetail = errorBody.errors.map((e: any) => e.msg).join(', ');
+          }
+        } catch {
+          /* Fallback to statusText if body isn't JSON */
+        }
+
+        throw new Error(`${response.status} ${errorDetail}`);
+      }


fetchWithTimeout() now parses the response body and throws when !response.ok, but the v3 client tests don't appear to exercise this behavior directly (e.g., ensuring an HTTP 4xx/5xx causes a rejection and that errors in the error body are surfaced). Adding a focused unit test for this branch will help prevent regressions and would have caught message/behavior changes in callers.

Signed-off-by: Simon Essien <champbreed1@gmail.com>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Champbreed <champbreed1@gmail.com>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-11T10:22:36Z

+        }
+
+        throw new Error(`${response.status} ${errorDetail}`);
+      }


Throwing only ${response.status} ${errorDetail} here changes the error message contract for callers like fetchServices/fetchSpanNames (they can no longer prepend context like "Failed to fetch services"), and will break the existing unit tests that assert those messages. Consider throwing a richer error (including URL) that callers can wrap, or catch/rethrow in the public methods to preserve contextual messages.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-11T21:57:02Z

+    } catch (error) {
+      throw new Error(`${context}: ${(error as Error).message}`);
+    }


The catch block wraps any failure with new Error(...), which drops the original stack trace and can also produce ...: undefined if a non-Error is thrown/rejected. Consider preserving the original error as cause and extracting the message via error instanceof Error ? error.message : String(error) (and avoid re-wrapping when the error is already contextualized).

Copilot · 2026-03-11T21:57:02Z

+    } catch (error) {
+      throw new Error(`${context}: ${(error as Error).message}`);
+    }


Same as fetchServices(): wrapping with new Error(...) loses the original stack/cause and (error as Error).message can be undefined for non-Error throwables. Prefer unknown + instanceof Error narrowing and preserve cause when adding context.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-11T22:01:25Z

@@ -36,34 +49,54 @@ export class JaegerClient {
   * @returns Promise<{ name: string; spanKind: string }[]> - Array of span name objects
   */
  async fetchSpanNames(service: string): Promise<{ name: string; spanKind: string }[]> {
-    const response = await this.fetchWithTimeout(
-      `${this.apiRoot}/operations?service=${encodeURIComponent(service)}`
-    );
-    if (!response.ok) {
-      throw new Error(
-        `Failed to fetch span names for service "${service}": ${response.status} ${response.statusText}`
+    const context = `Failed to fetch span names for service "${service}"`;
+    try {
+      const response = await this.fetchWithTimeout(
+        `${this.apiRoot}/operations?service=${encodeURIComponent(service)}`
      );
-    }
-    const data = await response.json();
+      const data = await response.json();
+
+      this.throwIfBodyHasErrors(data, context);



New behavior is introduced to throw on data.errors even for 200 OK responses, and to extract error messages from errors arrays in non-OK responses. There are existing unit tests for this client, but none that assert these new paths (e.g., ok: true with { errors: [...] }, or ok: false with { errors: [...] }) including the resulting error message. Adding tests would help prevent regressions of the infinite-loading fix.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-11T22:26:05Z

+      const data = await response.json();
+
+      this.throwIfBodyHasErrors(data);

-    // Runtime validation with Zod
-    const validated = ServicesResponseSchema.parse(data);
-    return validated.services;
+      const validated = ServicesResponseSchema.parse(data);


New behavior: fetchServices()/fetchSpanNames() now throw when the JSON body contains an errors array even if the HTTP status is 200. There are currently no v3 client unit tests covering this path; please add tests for (1) 200 OK with errors, and (2) non-OK responses with a JSON {errors:[...]} body to ensure the infinite-loading regression stays fixed.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Champbreed <champbreed1@gmail.com>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-11T22:57:01Z

+    } catch (error) {
+      const message = error instanceof Error ? error.message : String(error);
+      throw new Error(`${context}: ${message}`);
+    }


Same as fetchServices: this catch block wraps errors but drops the original error object/stack. Prefer attaching the original error via cause (or otherwise preserving it) so callers/debugging can inspect the underlying failure (HTTP error vs validation error vs timeout).

Copilot · 2026-03-11T22:57:02Z

+      const data = await response.json();
+
+      this.throwIfBodyHasErrors(data);

-    // Runtime validation with Zod
-    const validated = ServicesResponseSchema.parse(data);
-    return validated.services;
+      const validated = ServicesResponseSchema.parse(data);


New behavior: throwIfBodyHasErrors turns 200 OK responses with an errors array into failures. There are existing unit tests for this client, but none cover the 200-with-errors case. Add tests for both fetchServices and fetchSpanNames where ok: true and the JSON body contains a non-empty errors array, asserting the promise rejects and the message is surfaced.

Champbreed requested a review from a team as a code owner March 11, 2026 07:14

Copilot AI review requested due to automatic review settings March 11, 2026 07:14

Copilot started reviewing on behalf of Champbreed March 11, 2026 07:14 View session